[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] [Intel] Backport bugfixes for SRF LBR branch counter#838
Merged
Conversation
added 2 commits
June 2, 2025 12:59
[ Upstream commit 75aea4b ] A warning in intel_pmu_lbr_counters_reorder() may be triggered by below perf command. perf record -e "{cpu-clock,cycles/call-graph="lbr"/}" -- sleep 1 It's because the group is mistakenly treated as a branch counter group. The hw.flags of the leader are used to determine whether a group is a branch counters group. However, the hw.flags is only available for a hardware event. The field to store the flags is a union type. For a software event, it's a hrtimer. The corresponding bit may be set if the leader is a software event. For a branch counter group and other groups that have a group flag (e.g., topdown, PEBS counters snapshotting, and ACR), the leader must be a X86 event. Check the X86 event before checking the flag. The patch only fixes the issue for the branch counter group. The following patch will fix the other groups. There may be an alternative way to fix the issue by moving the hw.flags out of the union type. It should work for now. But it's still possible that the flags will be used by other types of events later. As long as that type of event is used as a leader, a similar issue will be triggered. So the alternative way is dropped. Fixes: 3374491 ("perf/x86/intel: Support branch counters logging") Closes: https://lore.kernel.org/lkml/20250412091423.1839809-1-luogengkun@huaweicloud.com/ Reported-by: Luo Gengkun <luogengkun@huaweicloud.com> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/20250424134718.311934-2-kan.liang@linux.intel.com [ Backport from v6.15 ] Suggested-by: Wentao Guan <guanwentao@uniontech.com> Signed-off-by: WangYuli <wangyuli@uniontech.com>
[ Upstream commit ef493f4 ] The BPF subsystem may capture LBR data on a counting event. However, the current implementation assumes that LBR can/should only be used with sampling events. For instance, retsnoop tool ([0]) makes an extensive use of this functionality and sets up perf event as follows: struct perf_event_attr attr; memset(&attr, 0, sizeof(attr)); attr.size = sizeof(attr); attr.type = PERF_TYPE_HARDWARE; attr.config = PERF_COUNT_HW_CPU_CYCLES; attr.sample_type = PERF_SAMPLE_BRANCH_STACK; attr.branch_sample_type = PERF_SAMPLE_BRANCH_KERNEL; To limit the LBR for a sampling event is to avoid unnecessary branch stack setup for a counting event in the sample read. Because LBR is only read in the sampling event's overflow. Although in most cases LBR is used in sampling, there is no HW limit to bind LBR to the sampling mode. Allow an LBR setup for a counting event unless in the sample read mode. Fixes: 85846b2 ("perf/x86: Add PERF_X86_EVENT_NEEDS_BRANCH_STACK flag") Closes: https://lore.kernel.org/lkml/20240905180055.1221620-1-andrii@kernel.org/ Reported-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Andrii Nakryiko <andrii@kernel.org> Tested-by: Andrii Nakryiko <andrii@kernel.org> Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20240909155848.326640-1-kan.liang@linux.intel.com [ Backport from v6.12 ] Suggested-by: Wentao Guan <guanwentao@uniontech.com> Signed-off-by: WangYuli <wangyuli@uniontech.com>
Reviewer's GuideBackport SRF LBR branch counter bugfixes by exposing the is_x86_event API, centralizing leader flag checks into a helper, refactoring branch-group detection, and tightening branch-stack flag assignment to skip count-only sample reads. Sequence Diagram for Updated is_branch_counters_group LogicsequenceDiagram
participant Caller
participant is_branch_counters_group
participant check_leader_group
participant is_x86_event
Caller->>is_branch_counters_group: event
is_branch_counters_group->>check_leader_group: event->group_leader, PERF_X86_EVENT_BRANCH_COUNTERS
check_leader_group->>is_x86_event: leader
is_x86_event-->>check_leader_group: bool (is_x86_event_result)
alt is_x86_event_result is true
check_leader_group->>check_leader_group: Check leader->hw.flags & flags
end
check_leader_group-->>is_branch_counters_group: bool (check_leader_group_result)
is_branch_counters_group-->>Caller: bool (final_result)
Class Diagram of Changes to X86 Performance Event HandlingclassDiagram
class X86EventFunctions {
<<Module: x86 Events>>
+is_x86_event(event: perf_event*) bool // Exposed
+check_leader_group(leader: perf_event*, flags: int) bool // New helper
+is_branch_counters_group(event: perf_event*) bool // Modified
+intel_pmu_hw_config(event: perf_event*) int // Modified
}
class perf_event {
<<struct>>
+hw_flags : int
+attr_sample_type : int
+group_leader : perf_event*
}
X86EventFunctions ..> perf_event : Uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull Request Overview
This pull request backports bugfixes for Intel SRF LBR branch counter handling in the x86 perf event subsystem.
- Introduces
is_x86_eventandcheck_leader_groupto guard branch-counter queries on non-x86 events - Refactors
is_branch_counters_groupto use the new helper - Adjusts branch stack setup logic to avoid unintended counting behavior during SAMPLE_READ
- Promotes
is_x86_eventfromstatic inlineto an external function
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| arch/x86/events/perf_event.h | Declare is_x86_event externally and add check_leader_group helper |
| arch/x86/events/intel/core.c | Update branch stack setup conditions with nested logic |
| arch/x86/events/core.c | Move is_x86_event out of static inline and into external linkage |
Comments suppressed due to low confidence (1)
arch/x86/events/perf_event.h:115
- The
flagsparameter is declared asintbuthw.flagsis likely an unsigned long bitmask; consider changingflagstounsigned longto match and avoid type mismatches.
static inline bool check_leader_group(struct perf_event *leader, int flags)
Comment on lines
+4008
to
+4009
| !(event->attr.sample_type & PERF_SAMPLE_READ)) | ||
| event->hw.flags |= PERF_X86_EVENT_NEEDS_BRANCH_STACK; |
There was a problem hiding this comment.
[nitpick] Consider adding braces around the inner if block to improve readability and prevent accidental inclusion of additional statements in future edits.
Suggested change
| !(event->attr.sample_type & PERF_SAMPLE_READ)) | |
| event->hw.flags |= PERF_X86_EVENT_NEEDS_BRANCH_STACK; | |
| !(event->attr.sample_type & PERF_SAMPLE_READ)) { | |
| event->hw.flags |= PERF_X86_EVENT_NEEDS_BRANCH_STACK; | |
| } |
| return is_metric_event(event) || is_slots_event(event); | ||
| } | ||
|
|
||
| int is_x86_event(struct perf_event *event); |
There was a problem hiding this comment.
Add a brief comment describing what constitutes an "x86 event" so maintainers understand the criteria without digging into the implementation.
| } | ||
|
|
||
| static inline int is_x86_event(struct perf_event *event) | ||
| int is_x86_event(struct perf_event *event) |
There was a problem hiding this comment.
[nitpick] Since is_x86_event returns a boolean-like result, consider changing its return type to bool for clarity and consistency with other predicate functions.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Avenger-285714 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (needs_branch_stack(event) && is_sampling_event(event)) | ||
| event->hw.flags |= PERF_X86_EVENT_NEEDS_BRANCH_STACK; | ||
| if (needs_branch_stack(event)) { | ||
| /* Avoid branch stack setup for counting events in SAMPLE READ */ |
There was a problem hiding this comment.
suggestion: Clarify SAMPLE_READ logic in comment
Consider updating the comment to clarify that branch stack setup is skipped only when the event is not a sampling event and PERF_SAMPLE_READ is set.
Suggested change
| /* Avoid branch stack setup for counting events in SAMPLE READ */ | |
| /* Skip branch stack setup only for non-sampling events with PERF_SAMPLE_READ set */ |
deepin pr auto review关键摘要:
是否建议立即修改:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: #828
Suggested-by: Wentao Guan guanwentao@uniontech.com
Summary by Sourcery
Backport Intel SRF LBR branch counter bug fixes to x86 perf event handling in kernel 6.6
Bug Fixes:
Enhancements: